feat(sandbox): add Rscript support to environment configuration#490
feat(sandbox): add Rscript support to environment configuration#490zieen wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
Add documentation and test coverage for Rscript language support in sandbox child process environment variables. Rscript benefits from existing lowercase proxy variables and libcurl/OpenSSL TLS certificate bundles. Changes: - Document Rscript compatibility with http_proxy/https_proxy - Document SSL_CERT_FILE coverage for R and other OpenSSL tools - Document CURL_CA_BUNDLE for R httr package - Add test assertions verifying lowercase proxy vars - Add test assertion verifying CURL_CA_BUNDLE
|
Thank you for your interest in contributing to OpenShell, @zieen. This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer. To get vouched:
See CONTRIBUTING.md for details. |
|
Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot. |
There was a problem hiding this comment.
Pull request overview
This PR improves sandbox child-process environment configuration clarity and coverage for Rscript by documenting and testing the relevant proxy and TLS certificate bundle environment variables.
Changes:
- Document Rscript compatibility with lowercase
http_proxy/https_proxyand libcurl/OpenSSL bundle env vars. - Extend unit tests to assert lowercase proxy variables are present.
- Extend TLS env var test to assert
CURL_CA_BUNDLEis set.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Node.js only honors HTTP(S)_PROXY for built-in fetch/http clients when | ||
| // proxy support is explicitly enabled at process startup. | ||
| // Rscript automatically respects lowercase http_proxy/https_proxy. | ||
| ("NODE_USE_ENV_PROXY", "1".to_owned()), |
There was a problem hiding this comment.
The new Rscript note is placed immediately above NODE_USE_ENV_PROXY, which could imply it relates to that setting. Since it documents the lowercase http_proxy/https_proxy entries, consider moving this comment next to those env var entries (or splitting into separate comments for Node vs. lowercase proxies) to keep the rationale aligned with the relevant keys.
Add documentation and test coverage for Rscript language support in sandbox child process environment variables. Rscript benefits from existing lowercase proxy variables and libcurl/OpenSSL TLS certificate bundles.
Changes:
Summary
Related Issue
Changes
Testing
mise run pre-commitpassesChecklist